Skip to content

Add getters for 'demand' and 'controlMode' in FakeMotor#77

Merged
kcooney merged 1 commit into
mainfrom
kcooney/fake-motor-fix
Oct 14, 2025
Merged

Add getters for 'demand' and 'controlMode' in FakeMotor#77
kcooney merged 1 commit into
mainfrom
kcooney/fake-motor-fix

Conversation

@kcooney
Copy link
Copy Markdown
Contributor

@kcooney kcooney commented Oct 13, 2025

This is in preparation for making the 'demand' field package-scope. Now that FakeMotor has an 'isStopped' field, allowing code outside of FakeMotor to directly update the value is error-prone.

This is in preparation for making the 'demand' field package-scope. Now that
FakeMotor has an 'isStopped' field, allowing code outside of FakeMotor to
directly update the value is error-prone.
@kcooney kcooney requested a review from vdikov October 13, 2025 03:17
private ControlMode controlMode = ControlMode.VOLTAGE;

/** Gets the most recently applied demand value for this motor. */
public final double getDemand() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering the other day, if we want to make the Motor abstraction be a generic that takes the control mode as a template parameter. Then, the getDemand() should return demand in the type that matches the control mode - Position, Velocity, Voltage, etc.

This way, we could potentially prevent improper uses of the demand value (e.g., use a demand value as a voltage, from a motor that has been set up for speed).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vdikov That's an interesting idea. Perhaps create an issue tracking that?

@kcooney kcooney merged commit 47c1662 into main Oct 14, 2025
1 check passed
@kcooney kcooney deleted the kcooney/fake-motor-fix branch October 19, 2025 00:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants